Document soundness of Integer -> Pointer -> Integer conversions in `const` contexts. #113510

onestacked · 2023-07-09T16:39:10Z

With this slice Iterator's should be able to be made const once the const Trait reimplementation is done.

RalfJung · 2023-07-09T17:23:07Z

LGTM. However, I think we need the lang team to bless this. Nominating for their discussion.
Cc @rust-lang/wg-const-eval @rust-lang/lang

onestacked · 2023-07-10T14:54:40Z

@RalfJung Are there any const operations that can be done on the pointer (between the transmutes) that would make this unsound?
I can't think of any, but maybe it would be good to restrict this in some way for now.

RalfJung · 2023-07-10T18:52:07Z

No, in const you can never turn an integer value into a pointer value.

joshtriplett · 2023-08-01T18:57:58Z

@rfcbot merge

@scottmcm has a suggestion/concern he's going to raise, but since we know this needs an FCP I'm going ahead and starting one.

rfcbot · 2023-08-01T18:58:00Z

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

~~concept-vs-method~~ resolved by Document soundness of Integer -> Pointer -> Integer conversions in const contexts. #113510 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

library/core/src/intrinsics.rs

scottmcm · 2023-08-01T19:15:04Z

Absolutely in favour in the abstract 👍

I do think, though, that we want it to be able to general idea of transmuting, rather than limiting the text to allowing it only via mem::transmute and ptr::invalid. (For example, mem::transmute_copy feels like it ought not to be UB.)

I've proposed some alternative text above to kick of discussion about the best way to phrase such a guarantee.
@rfcbot concern concept-vs-method

@rfcbot reviewed

scottmcm · 2023-08-01T19:49:58Z

@rfcbot resolve concept-vs-method

Though, other folks, please review the text to ensure it's still something with which you're ok.

rfcbot · 2023-08-01T19:50:05Z

🔔 This is now entering its final comment period, as per the review above. 🔔

RalfJung · 2023-08-02T09:05:28Z

library/core/src/intrinsics.rs

+    /// Transmuting pointers *to* integers in a `const` context is [undefined behavior][ub],
+    /// unless the pointer was originally created by transmuting *from* an integer.
+    /// (That includes this function specifically and helpers like [`invalid`][crate::ptr::invalid],
+    /// but also semantically-equivalent conversions such as punning through `repr(C)` union fields.)


Suggested change

/// Transmuting pointers *to* integers in a `const` context is [undefined behavior][ub],

/// unless the pointer was originally created by transmuting *from* an integer.

/// (That includes this function specifically and helpers like [`invalid`][crate::ptr::invalid],

/// but also semantically-equivalent conversions such as punning through `repr(C)` union fields.)

/// Transmuting pointers *to* integers in a `const` context is [undefined behavior][ub],

/// unless the pointer was originally created *from* an integer.

/// (The pointer can be created from an integer via integer-to-pointer casts, helpers like [`invalid`][crate::ptr::invalid],

/// this function, or semantically-equivalent conversions such as punning through `repr(C)` union fields.)

We don't want to say that invalid is a transmute. We just want to say that transmute(ptr::invalid(N)): *mut T is allowed, as is transmute(Nusize): *mut T.

Wait, does "that includes" refer to "created by" or to "transmuting to"? It must be "created by" as otherwise invalid can't be in the list, but then this doesn't cover union field punning from invalid to an integer.

IMO this section shouldn't talk specifically about that anyway, that should fall out of the general principle that transmute and union field punning are equivalent.

Maybe I should mention only [invalid][crate::ptr::invalid] explicitly?

Something like:

unless the pointer was originally converted *from* an integer. (for example with this function or [`invalid`][crate::ptr::invalid])

I'm pretty sure we want to allow transmuting 16 as *const u8 back to an integer. There is really no reason not to.

I would say that is converted from and integer and therefore allowed.
Not sure how to write a concise description that makes it clear all the possibilities for this conversion are allowed.

The parenthetical explains what it means to convert from an integer. So then if you think casting is converting from an integer, it should be listed. :)

…const` contexts. see this [zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/Soundness.20of.20Integer.20-.3E.20Pointer.20-.3E.20Integer.20conversions)

onestacked · 2023-08-02T12:06:52Z

Applied @RalfJung's suggestions.

scottmcm · 2023-08-03T05:21:56Z

(Re-nominated to encourage discussion of the exact text on Tuesday.)

rfcbot · 2023-08-11T19:51:09Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

RalfJung · 2023-09-05T12:27:06Z

Oh, sorry, I didn't realize this was assigned to me...

@bors r+ rollup

bors · 2023-09-05T12:27:08Z

📌 Commit dc4e026 has been approved by RalfJung

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#113510 (Document soundness of Integer -> Pointer -> Integer conversions in `const` contexts.) - rust-lang#114412 (document our assumptions about symbols provided by the libc) - rust-lang#114813 (explain why we can mutate the FPU control word) - rust-lang#115523 (improve `AttrTokenStream`) - rust-lang#115536 (interpret: make MemPlace, Place, Operand types private to the interpreter) - rust-lang#115540 (Support debuginfo for custom MIR.) - rust-lang#115563 (llvm-wrapper: adapt for LLVM API change) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned RalfJung Jul 9, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 9, 2023

onestacked force-pushed the const_ptr_transmute_docs branch from e4b522d to 1c20573 Compare July 9, 2023 16:46

joshtriplett removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Aug 1, 2023

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 1, 2023

scottmcm reviewed Aug 1, 2023

View reviewed changes

library/core/src/intrinsics.rs Outdated Show resolved Hide resolved

onestacked force-pushed the const_ptr_transmute_docs branch from 1c20573 to 8af820d Compare August 1, 2023 19:44

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Aug 1, 2023

RalfJung reviewed Aug 2, 2023

View reviewed changes

Document soundness of Integer -> Pointer -> Integer conversions in ` …

dc4e026

…const` contexts. see this [zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/Soundness.20of.20Integer.20-.3E.20Pointer.20-.3E.20Integer.20conversions)

onestacked force-pushed the const_ptr_transmute_docs branch from 8af820d to dc4e026 Compare August 2, 2023 12:05

scottmcm added the I-lang-nominated Nominated for discussion during a lang team meeting. label Aug 3, 2023

rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Aug 11, 2023

rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Aug 11, 2023

apiraino removed the to-announce Announce this issue on triage meeting label Aug 18, 2023

RalfJung mentioned this pull request Aug 19, 2023

Specify bit validity and padding of some types rust-lang/reference#1392

Merged

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2023

matthiaskrgr mentioned this pull request Sep 5, 2023

Rollup of 7 pull requests #115568

Merged

bors merged commit a23f216 into rust-lang:master Sep 5, 2023
11 checks passed

rustbot added this to the 1.74.0 milestone Sep 5, 2023

onestacked deleted the const_ptr_transmute_docs branch September 6, 2023 14:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document soundness of Integer -> Pointer -> Integer conversions in `const` contexts. #113510

Document soundness of Integer -> Pointer -> Integer conversions in `const` contexts. #113510

onestacked commented Jul 9, 2023 •

edited

Loading

RalfJung commented Jul 9, 2023 •

edited

Loading

onestacked commented Jul 10, 2023

RalfJung commented Jul 10, 2023

joshtriplett commented Aug 1, 2023

rfcbot commented Aug 1, 2023 •

edited

Loading

scottmcm commented Aug 1, 2023

scottmcm commented Aug 1, 2023

rfcbot commented Aug 1, 2023

RalfJung Aug 2, 2023 •

edited

Loading

RalfJung Aug 2, 2023 •

edited

Loading

onestacked Aug 2, 2023

RalfJung Aug 2, 2023

onestacked Aug 2, 2023

RalfJung Aug 2, 2023

onestacked commented Aug 2, 2023

scottmcm commented Aug 3, 2023

rfcbot commented Aug 11, 2023

RalfJung commented Sep 5, 2023

bors commented Sep 5, 2023

Document soundness of Integer -> Pointer -> Integer conversions in const contexts. #113510

Document soundness of Integer -> Pointer -> Integer conversions in const contexts. #113510

Conversation

onestacked commented Jul 9, 2023 • edited Loading

RalfJung commented Jul 9, 2023 • edited Loading

onestacked commented Jul 10, 2023

RalfJung commented Jul 10, 2023

joshtriplett commented Aug 1, 2023

rfcbot commented Aug 1, 2023 • edited Loading

scottmcm commented Aug 1, 2023

scottmcm commented Aug 1, 2023

rfcbot commented Aug 1, 2023

RalfJung Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

RalfJung Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

onestacked Aug 2, 2023

Choose a reason for hiding this comment

RalfJung Aug 2, 2023

Choose a reason for hiding this comment

onestacked Aug 2, 2023

Choose a reason for hiding this comment

RalfJung Aug 2, 2023

Choose a reason for hiding this comment

onestacked commented Aug 2, 2023

scottmcm commented Aug 3, 2023

rfcbot commented Aug 11, 2023

RalfJung commented Sep 5, 2023

bors commented Sep 5, 2023

Document soundness of Integer -> Pointer -> Integer conversions in `const` contexts. #113510

Document soundness of Integer -> Pointer -> Integer conversions in `const` contexts. #113510

onestacked commented Jul 9, 2023 •

edited

Loading

RalfJung commented Jul 9, 2023 •

edited

Loading

rfcbot commented Aug 1, 2023 •

edited

Loading

RalfJung Aug 2, 2023 •

edited

Loading

RalfJung Aug 2, 2023 •

edited

Loading